Skip to content

fix(voice): rimuovi clearChunks ridondante + aggiorna commenti obsoleti#19

Merged
Gabry848 merged 2 commits intomasterfrom
fix/voice-cleanup-redundancy
Mar 9, 2026
Merged

fix(voice): rimuovi clearChunks ridondante + aggiorna commenti obsoleti#19
Gabry848 merged 2 commits intomasterfrom
fix/voice-cleanup-redundancy

Conversation

@Gabry848
Copy link
Copy Markdown
Collaborator

@Gabry848 Gabry848 commented Mar 7, 2026

Sommario

Pulizia codice in useVoiceChat.ts a seguito della refactor server-side (rimozione VoiceAudioGate).

Modifiche

Fix ridondanza — handler interrupted in onStatus:

  • stopPlayback() resetta già tutto lo stato dell'AudioPlayer (chunkCounter, pendingChunks, allChunksReceived, sessionId++)
  • Chiamare clearChunks() subito dopo causava un doppio sessionId++ inutile

Commenti aggiornati — rimossi riferimenti a "server gate in voice_bridge.py" che non esiste più dopo la refactor lato server (PR #81 su MyTaskly-server)

Commit

  1. fix(voice): rimozione clearChunks ridondante dopo stopPlayback
  2. refactor(voice): aggiornamento commenti obsoleti su server gate

Gabry848 and others added 2 commits March 7, 2026 15:00
…rupted

stopPlayback() resetta già tutto lo stato interno dell'AudioPlayer
(chunkCounter, pendingChunks, allChunksReceived, sessionId++).
Chiamare clearChunks() subito dopo causava un doppio incremento
di sessionId senza alcun beneficio.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Rimossi riferimenti al 'server gate in voice_bridge.py' che non esiste
più dopo la refactor lato server. Il VAD è ora gestito interamente
dal server-side VAD di OpenAI Realtime API.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@claude
Copy link
Copy Markdown

claude bot commented Mar 7, 2026

Code Review - overall a clean and focused PR.

@claude
Copy link
Copy Markdown

claude bot commented Mar 7, 2026

Code Review

Overall this is a clean, focused PR with a small diff and a clear rationale. Here is my analysis.


Main fix — removal of redundant clearChunks()

Correct. In src/utils/audioUtils.ts, stopPlayback() (~line 690) already does this.sessionId++ and resets chunkCounter, pendingChunks, and allChunksReceived to their initial values. clearChunks() (~line 683) does exactly the same thing. Calling clearChunks() right after stopPlayback() caused a double sessionId++. While this did not produce visible bugs (in-flight addChunk calls capture mySession synchronously before either increment, so they are already invalidated by the first stopPlayback() increment), it was semantically wrong and redundant. Removing it is the right call.


Comment updates

Both changes are accurate:

  1. speech_stopped case: the old comment referenced a server gate in voice_bridge.py that no longer exists after the server-side refactor (PR 81 on MyTaskly-server). The new wording correctly attributes turn management to OpenAI VAD.
  2. onChunk callback: same fix for the same reason — mic gating moved from voice_bridge.py to OpenAI server-side VAD. Accurate and avoids misleading future maintainers.

Minor suggestions (non-blocking)

  • clearChunks() is still exposed on AudioPlayer. Now that stopPlayback() covers all the same resets, it is worth checking whether clearChunks() is called anywhere else in the codebase. If not, removing it in a follow-up would reduce the API surface and eliminate the class of confusion that led to this bug.

  • The interrupted handler sets agentEndedRef.current = true, which is correct and intentional (prevents the audio_end callback from transitioning back to processing for the old turn). This is unrelated to this PR, but a short inline comment explaining the why would help future readers.


Summary: bug fix is correct, comment updates are accurate, scope is focused with no unrelated changes, no security concerns. Approved.

@Gabry848 Gabry848 merged commit 2fb523e into master Mar 9, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant